Skip to content

Conversation

@pwilkin
Copy link
Collaborator

@pwilkin pwilkin commented Sep 18, 2025

EDIT: README FIRST
This is an implementation of a new type of attention gating in GGML.
Therefore, this implementation will be focused on CORRECTNESS ONLY.
Speed tuning and support for more architectures will come in future PRs.
Please do not spam this threads with reports about performance, especially on backend architectures (CUDA, Vulkan).

CURRENT STATE: core is done

===
It's been a real learning experience, not gonna lie, but if someone with hybrid model implementation experience (@gabe-l-hart ?) has some quick tips, I'd be grateful.

Resolves #15940

@github-actions github-actions bot added python python script changes ggml changes relating to the ggml tensor library for machine learning labels Sep 18, 2025
@gabe-l-hart
Copy link
Collaborator

I'll try to get into it in more detail soon, but here are a few general thoughts after quickly skimming the PR:

  1. The structure of what you've got smells correct, so it's likely close, but missing something small yet critical
  2. A full repro with the error it's raising would definitely help debug
  3. My debugging process for this would be:
    1. Make sure tokenization is solid (print statements as necessary to compare tokens before input)
    2. Use llama-eval-callback to dump tensors for a single prefill step
    3. Run an identical single prefill with the reference impl (transformers or otherwise), and inject prints as needed to dump tensors along the way
    4. Visually comb through them (particularly the sum at each point) to see where things start diverging significantly

@bugparty
Copy link
Contributor

It's been a real learning experience, not gonna lie, but if someone with hybrid model implementation experience (@gabe-l-hart ?) has some quick tips, I'd be grateful.

Currently at the stage of "graph builds, but first decode complains about wrong memory model", probably not building the inputs correctly.

Resolves #15940

interesting, maybe we can learn together

@pwilkin pwilkin marked this pull request as draft September 19, 2025 08:07
@pwilkin
Copy link
Collaborator Author

pwilkin commented Sep 19, 2025

  1. A full repro with the error it's raising would definitely help debug

Running llama-cli -m reference/qwen3_next_500m/Qwen3_Next_500M-8x417M-BF16.gguf -ngl 999 -p "Who are " yields this weird memory error:

#0  __syscall_cancel_arch () at ../sysdeps/unix/sysv/linux/x86_64/syscall_cancel.S:56
56      in ../sysdeps/unix/sysv/linux/x86_64/syscall_cancel.S
#1  0x000070552b29eb63 in __internal_syscall_cancel (a1=<optimized out>, a2=<optimized out>, a3=<optimized out>, a4=<optimized out>, a5=0, a6=0, nr=61) at ./nptl/cancellation.c:49
warning: 49     ./nptl/cancellation.c: No such file or directory
#2  __syscall_cancel (a1=<optimized out>, a2=<optimized out>, a3=<optimized out>, a4=<optimized out>, a5=a5@entry=0, a6=a6@entry=0, nr=61) at ./nptl/cancellation.c:75
75      in ./nptl/cancellation.c
#3  0x000070552b31afdf in __GI___wait4 (pid=<optimized out>, stat_loc=<optimized out>, options=<optimized out>, usage=<optimized out>) at ../sysdeps/unix/sysv/linux/wait4.c:30
warning: 30     ../sysdeps/unix/sysv/linux/wait4.c: No such file or directory
#4  0x000070552bb45c31 in ggml_print_backtrace () at /devel/tools/llama.cpp/ggml/src/ggml.c:196
warning: Source file is more recent than executable.
196             waitpid(child_pid, NULL, 0);
#5  0x000070552bb45de5 in ggml_abort (file=0x70552bbcdac8 "/devel/tools/llama.cpp/ggml/src/ggml-backend.cpp", line=189, fmt=0x70552bbcd8af "GGML_ASSERT(%s) failed") at /devel/tools/llama.cpp/ggml/src/ggml.c:230
230             ggml_print_backtrace();
#6  0x000070552bb6091e in ggml_backend_buffer_get_type (buffer=0x0) at /devel/tools/llama.cpp/ggml/src/ggml-backend.cpp:189
189         GGML_ASSERT(buffer);
#7  0x000070552bb6080e in ggml_backend_buffer_is_host (buffer=0x0) at /devel/tools/llama.cpp/ggml/src/ggml-backend.cpp:170
170         return ggml_backend_buft_is_host(ggml_backend_buffer_get_type(buffer));
#8  0x000070552c07a114 in llm_graph_input_rs::set_input (this=0x5f11bdf6aea0, ubatch=0x5f11be011300) at /devel/tools/llama.cpp/src/llama-graph.cpp:241
241             GGML_ASSERT(ggml_backend_buffer_is_host(s_copy->buffer));
#9  0x000070552c07b03c in llm_graph_input_mem_hybrid::set_input (this=0x5f11bdf6aee0, ubatch=0x5f11be011300) at /devel/tools/llama.cpp/src/llama-graph.cpp:437
437         inp_rs->set_input(ubatch);
#10 0x000070552c07b549 in llm_graph_result::set_inputs (this=0x5f11be01ddf0, ubatch=0x5f11be011300) at /devel/tools/llama.cpp/src/llama-graph.cpp:480
480             input->set_input(ubatch);
#11 0x000070552c01ddb3 in llama_context::process_ubatch (this=0x5f11c05b5b50, ubatch=..., gtype=LLM_GRAPH_TYPE_DECODER, mctx=0x5f11be00ff00, ret=@0x7fff74d22ea4: 538976288) at /devel/tools/llama.cpp/src/llama-context.cpp:779
779             res->set_inputs(&ubatch);
#12 0x000070552c01f367 in llama_context::decode (this=0x5f11c05b5b50, batch_inp=...) at /devel/tools/llama.cpp/src/llama-context.cpp:1088
1088            const auto * res = process_ubatch(ubatch, LLM_GRAPH_TYPE_DECODER, mctx.get(), status);
#13 0x000070552c025e49 in llama_decode (ctx=0x5f11c05b5b50, batch=...) at /devel/tools/llama.cpp/src/llama-context.cpp:2726
2726        const int ret = ctx->decode(batch);
#14 0x00005f11a2021559 in common_init_from_params (params=...) at /devel/tools/llama.cpp/common/common.cpp:1066
1066                llama_decode(lctx, llama_batch_get_one(tmp.data(), std::min(tmp.size(), (size_t) params.n_batch)));
#15 0x00005f11a1e4a3c0 in main (argc=7, argv=0x7fff74d25968) at /devel/tools/llama.cpp/tools/main/main.cpp:140
140         common_init_result llama_init = common_init_from_params(params);

I'll try to merge the op into the ggml_delta_net function call as @ngxson suggested.

@CISC
Copy link
Collaborator

CISC commented Sep 19, 2025

  1. A full repro with the error it's raising would definitely help debug

Running llama-cli -m reference/qwen3_next_500m/Qwen3_Next_500M-8x417M-BF16.gguf -ngl 999 -p "Who are " yields this weird memory error:

...
#6  0x000070552bb6091e in ggml_backend_buffer_get_type (buffer=0x0) at /devel/tools/llama.cpp/ggml/src/ggml-backend.cpp:189
189         GGML_ASSERT(buffer);
#7  0x000070552bb6080e in ggml_backend_buffer_is_host (buffer=0x0) at /devel/tools/llama.cpp/ggml/src/ggml-backend.cpp:170
170         return ggml_backend_buft_is_host(ggml_backend_buffer_get_type(buffer));
...

The backend buffer is NULL.

@ngxson
Copy link
Collaborator

ngxson commented Sep 19, 2025

#9  0x000070552c07b03c in llm_graph_input_mem_hybrid::set_input (this=0x5f11bdf6aee0, ubatch=0x5f11be011300) at /devel/tools/llama.cpp/src/llama-graph.cpp:437
437         inp_rs->set_input(ubatch);

The model doesn't seem to have any recurrence layers. This makes the set input fails due to input node not being present in cgraph.

I'll try to merge the op into the ggml_delta_net function call as @ngxson suggested.

Hmm I think I said the reverse: not to merge it but make the op simple

I feel like this op can be implemented using other ggml ops like mul, mul_mat, sum. Which part of the calculation do you think that can't be constructed using existing ops?

This is the more important question: should we try to implement it using existing ops, or add a new op and spend even more time to optimize it cross all backends?

@pwilkin
Copy link
Collaborator Author

pwilkin commented Sep 19, 2025

Now this is an error I haven't expected to encounter:

GGML_ABORT("not enough space in the context's memory pool");

@pwilkin
Copy link
Collaborator Author

pwilkin commented Sep 19, 2025

The model doesn't seem to have any recurrence layers. This makes the set input fails due to input node not being present in cgraph.

How do I allocate the memory for the linear layers then? I seem to have misunderstood how build_inp_mem_hybrid() works...

@yarikdevcom
Copy link

@pwilkin any chance to buy you a coffee?(Paterson etc.) so community able to donate for your efforts. Thank you!

@pwilkin
Copy link
Collaborator Author

pwilkin commented Sep 19, 2025

@pwilkin any chance to buy you a coffee?(Paterson etc.) so community able to donate for your efforts. Thank you!

Added a buymeacoffee link to my profile (do consider first funding the Llama.cpp project itself, though!)

@ServeurpersoCom
Copy link
Collaborator

ServeurpersoCom commented Sep 19, 2025

@pwilkin any chance to buy you a coffee?(Paterson etc.) so community able to donate for your efforts. Thank you!

Added a buymeacoffee link to my profile (do consider first funding the Llama.cpp project itself, though!)

I send a coffee also.

@ngxson
Copy link
Collaborator

ngxson commented Sep 20, 2025

GGML_ABORT("not enough space in the context's memory pool");

Probably there are too many nodes on cgraph, try increasing the limit via llama_context::graph_max_nodes()

Comment on lines 19054 to 19056
Qcur = ggml_reshape_3d(ctx0, ggml_cont(ctx0, Qcur), n_embd_head, hparams.n_head(il), n_tokens);
Kcur = ggml_reshape_3d(ctx0, ggml_cont(ctx0, Kcur), n_embd_head, hparams.n_head_kv(il), n_tokens);
Vcur = ggml_reshape_3d(ctx0, ggml_cont(ctx0, Vcur), n_embd_head, hparams.n_head_kv(il), n_tokens);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these ggml_cont can be removed if Q/gate are separated. ggml_cont is not recommended when dealing with big tensors

Copy link
Collaborator

@CISC CISC Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually none of these need ggml_cont, Q is 3D already, Q/K are RoPEd so can be views and V can also be a 3D view now.

Edit: sorry, not quite true about V, only if QKV is fused, the weird gate fuse threw me off. Nevertheless, K/V are already contiguous at this point.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that Q is non-contiguous and ggml_rope(_ext) does not work very well with non-cont tensors, it's still buggy on certain backends

Copy link
Collaborator

@CISC CISC Sep 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that Q is non-contiguous and ggml_rope(_ext) does not work very well with non-cont tensors, it's still buggy on certain backends

Are you sure? AFAIK those issues are fixed.

Edit: Also, if there still are issues they will never get fixed if we work around them. :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that Q is non-contiguous and ggml_rope(_ext) does not work very well with non-cont tensors, it's still buggy on certain backends

I think all of these cases are fixed now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an impl of 2D rope that relies on ggml_view: https://github.com/ngxson/ggml-easy/blob/f56e5e499b1f21a4aae73010e9d9582840428457/demo/2d-rope.cpp

It works on CPU and Metal, but doesn't work on CUDA/Vulkan. Couldn't tested on other backends, but feel free to make a PR to address this issue.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes that seems to work. sorry @pwilkin you will need to manually revert the change where I split Q/gate. the tensor shape for Q will be:

layer.wq = create_tensor(tn(LLM_TENSOR_ATTN_Q, "weight", i), { n_embd, n_embd_head_k * n_head * 2 }, 0);

layer.ssm_a = create_tensor(tn(LLM_TENSOR_SSM_A, i), { hparams.ssm_dt_rank }, 0);
layer.ssm_beta_alpha = create_tensor(tn(LLM_TENSOR_SSM_BETA_ALPHA, "weight", i), { n_embd, ba_projection_size }, 0);
layer.ssm_norm = create_tensor(tn(LLM_TENSOR_SSM_NORM, "weight", i), { head_v_dim }, 0);
layer.ssm_out = create_tensor(tn(LLM_TENSOR_SSM_OUT, "weight", i), { n_ff, n_embd }, 0);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shape of LLM_TENSOR_ATTN_Q and LLM_TENSOR_SSM_OUT should not contain n_ff

@ngxson
Copy link
Collaborator

ngxson commented Sep 20, 2025

^ proposed fix for the 3 comments above: 46110e0

@pwilkin
Copy link
Collaborator Author

pwilkin commented Sep 20, 2025

@ngxson Thanks, scale_bias was one op I was missing in my endeavors :>

I got an LLM to rewrite the internal delta into tensor logic. After a day of manually fixing that crap, I think I understand it enough to rewrite it myself ;)

@ngxson
Copy link
Collaborator

ngxson commented Sep 20, 2025

Honestly I would prefer taking time to understand the mamba/ssm implementation then writing the code manually. Code written by LLM are mostly attempts for 1-to-1 translation from pytorch --> GGML which looks quite confusing

@pwilkin
Copy link
Collaborator Author

pwilkin commented Sep 20, 2025

Honestly I would prefer taking time to understand the mamba/ssm implementation then writing the code manually. Code written by LLM are mostly attempts for 1-to-1 translation from pytorch --> GGML which looks quite confusing

Yeah, for me getting a rough outline then going over it manually is the best way to learn :)

I tried the "one-to-one" approach and ended up with a graph that wouldn't fit in 16 GB of RAM for a 500M model...

@pwilkin
Copy link
Collaborator Author

pwilkin commented Sep 20, 2025

Aight, I cleaned up the main graph calculation, now I have to figure out how to include conv_states_all in my delta_net function in order to not get the memory error.

@ggerganov
Copy link
Member

I think the graph implementation could be improved - my guess is there are a few extra conts and transposes that we can avoid at very least.

Regarding the chunking, apart from the many number of nodes, does it work correctly?

Can you adapt the implementation to support chunking and by default set it to 1 chunk? No need to do the padding from the original version - just check if the batch size is exact multiple of the chunk size and if it is not, then do a single chunk. This way it would be clearer what needs to be done to support chunking and help understand how the performance changes with chunk size.

@pwilkin
Copy link
Collaborator Author

pwilkin commented Nov 20, 2025

I think the graph implementation could be improved - my guess is there are a few extra conts and transposes that we can avoid at very least.

Yeah, I think that there must be a few more optimizations that can be done by pre-preparing the tensors in advance. Also, I think a split operation that chunks a tensor into contiguous blocks might be worthwhile here.

Regarding the chunking, apart from the many number of nodes, does it work correctly?

Good question, I went directly from the single-op implementation to the one-chunk-unified implementation, so didn't test it.

Can you adapt the implementation to support chunking and by default set it to 1 chunk? No need to do the padding from the original version - just check if the batch size is exact multiple of the chunk size and if it is not, then do a single chunk. This way it would be clearer what needs to be done to support chunking and help understand how the performance changes with chunk size.

Yup, I can do that. I'll implement the chunking and we'll see how well this can be optimized further (hopefully I don't run out of RAM on the way 😄 )

@pwilkin
Copy link
Collaborator Author

pwilkin commented Nov 20, 2025

@ggerganov so I implemented the chunking logic but it seems the allocator doesn't like the chunks (not really sure what's happening here, maybe you have an idea? my guess would be it doesn't like the nodes being reused a variable number of times in a loop)

#6  0x00007e924855e0a0 in ggml_gallocr_allocate_node (galloc=0x5ffc5abbb950, node=0x5ffc5d210b30, buffer_id=-1) at /devel/tools/llama.cpp/ggml/src/ggml-alloc.c:629
629         GGML_ASSERT(buffer_id >= 0);
#7  0x00007e924855e5e0 in ggml_gallocr_alloc_graph_impl (galloc=0x5ffc5abbb950, graph=0x5ffc5ab06f88, node_buffer_ids=0x5ffc5d4e5490, leaf_buffer_ids=0x5ffc5d6cd4a0) at /devel/tools/llama.cpp/ggml/src/ggml-alloc.c:725
725             ggml_gallocr_allocate_node(galloc, leaf, get_node_buffer_id(leaf_buffer_ids, i));
#8  0x00007e924855eb7d in ggml_gallocr_reserve_n (galloc=0x5ffc5abbb950, graph=0x5ffc5ab06f88, node_buffer_ids=0x5ffc5d4e5490, leaf_buffer_ids=0x5ffc5d6cd4a0) at /devel/tools/llama.cpp/ggml/src/ggml-alloc.c:845
845         ggml_gallocr_alloc_graph_impl(galloc, graph, node_buffer_ids, leaf_buffer_ids);
#9  0x00007e9248567aa4 in ggml_backend_sched_reserve (sched=0x5ffc5ab06e30, measure_graph=0x5ffc5d1b4ad0) at /devel/tools/llama.cpp/ggml/src/ggml-backend.cpp:1705
1705        if (!ggml_gallocr_reserve_n(sched->galloc, &sched->graph, sched->node_backend_ids, sched->leaf_backend_ids)) {
#10 0x00007e9248a4e9f7 in llama_context::graph_reserve (this=0x5ffc5d157ae0, n_tokens=512, n_seqs=1, n_outputs=512, mctx=0x5ffc5abc7070, split_only=false) at /devel/tools/llama.cpp/src/llama-context.cpp:1438

@pwilkin
Copy link
Collaborator Author

pwilkin commented Nov 21, 2025

Nevermind, setting them as inputs fixed it, though I had to increase GGML_SCHED_MAX_SPLIT_INPUTS. Testing it now, but the graph situation isn't actually that terrible:

llama_context: graph nodes = 9265 (with bs=512), 8473 (with bs=1)

@pwilkin
Copy link
Collaborator Author

pwilkin commented Nov 21, 2025

Now getting another error:

llama-perplexity: /devel/tools/llama.cpp/ggml/src/ggml-cuda/ggml-cuda.cu:3498: void evaluate_and_capture_cuda_graph(ggml_backend_cuda_context*, ggml_cgraph*, bool&, bool&, bool&): Assertion `node->buffer->buft == ggml_backend_cuda_buffer_type(cuda_ctx->device)' failed.

@pwilkin
Copy link
Collaborator Author

pwilkin commented Nov 21, 2025

Aight, final version with chunking done. For various reasons had to restore the padding mechanism :)

The ultimate result in terms of graph size is impressive, but I think passable somewhat:

llama_context: graph nodes  = 11425 (with bs=512), 8473 (with bs=1)
llama_context: graph splits = 228 (with bs=512), 235 (with bs=1)
image

@pwilkin
Copy link
Collaborator Author

pwilkin commented Nov 22, 2025

Performance stats:

prompt eval time =   48628.94 ms / 21083 tokens (    2.31 ms per token,   433.55 tokens per second)
       eval time =  168618.07 ms /  2875 tokens (   58.65 ms per token,    17.05 tokens per second)
      total time =  217247.01 ms / 23958 tokens

(note: SOLVE_TRI is not yet implemented on any other backend than CPU)

@pwilkin
Copy link
Collaborator Author

pwilkin commented Nov 22, 2025

@ggerganov okay, I think the beast has been slain, core implementation with chunking is done, time for possible optimizations :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

examples ggml changes relating to the ggml tensor library for machine learning model Model specific Nvidia GPU Issues specific to Nvidia GPUs python python script changes testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feature Request: Qwen3-Next support